Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

project_types (formerly work schemes) #1098

Merged
merged 49 commits into from
Jun 4, 2018
Merged

Conversation

owocki
Copy link
Contributor

@owocki owocki commented May 7, 2018

Description

Support for

  • traditional
  • cooperative
  • competitive

work schemes

and

  • permissionless
  • appproval required

see also - alisa's email copy doc

Still TODO

  • enforce limits on # ppl starting work
  • build in the approval mechanism for 'approval required' schemes
  • build both schemes into the UI for bounty details page
  • build gitcoin bot comment updates
  • for contests, make it so it's open even if it's started
  • approval required as a default?
  • if you approve quickly… what if not approved quickly?
  • - - - system warns funder after 48 hours
  • - - - system auto approves you after 72 hours
  • email notifications upon approval
  • rename work scheme / application scheme to project type / permissions

to release

  • QA the shit out of this
  • what else?
Checklist
  • linter status: 100% pass
  • changes don't break existing behavior
  • commit message follows commit guidelines
Affected core subsystem(s)

all of it

Testing

tested it

Refers/Fixes

#973

'tech_stack'
'tech_stack',
'work_scheme',
'application_scheme',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected trailing comma. (comma-dangle)

@@ -452,8 +454,8 @@ var refreshBounties = function(event) {
result.action = result['url'];
result['title'] = result['title'] ? result['title'] : result['github_url'];


result['p'] = ((result['experience_level'] ? result['experience_level'] : 'Unknown Experience Level') + ' • ');
var work_scheme = ucwords(result['work_scheme']) + ' • ';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected blank line after variable declarations. (newline-after-var)

@@ -137,6 +137,10 @@ $(document).ready(function() {
githubUsername: metadata.githubUsername,
address: '' // Fill this in later
},
schemes: {
work_scheme: data.work_scheme,
application_scheme: data.application_scheme,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected trailing comma. (comma-dangle)

@@ -95,6 +95,12 @@ var sanitizeAPIResults = function(results) {
return results;
};

function ucwords (str) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected space before function parentheses. (space-before-function-paren)

@@ -95,6 +95,12 @@ var sanitizeAPIResults = function(results) {
return results;
};

function ucwords (str) {
return (str + '').replace(/^([a-z])|\s+([a-z])/g, function ($1) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected indentation of 2 spaces but found 4. (indent)
Unexpected space before function parentheses. (space-before-function-paren)

@@ -95,6 +95,12 @@ var sanitizeAPIResults = function(results) {
return results;
};

function ucwords (str) {
return (str + '').replace(/^([a-z])|\s+([a-z])/g, function ($1) {
return $1.toUpperCase();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected indentation of 4 spaces but found 8. (indent)

function ucwords (str) {
return (str + '').replace(/^([a-z])|\s+([a-z])/g, function ($1) {
return $1.toUpperCase();
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected indentation of 2 spaces but found 4. (indent)

@@ -118,7 +119,8 @@ def get_queryset(self):

# filtering
for key in ['raw_data', 'experience_level', 'project_length', 'bounty_type', 'bounty_owner_address',
'idx_status', 'network', 'bounty_owner_github_username', 'standard_bounties_id']:
'idx_status', 'network', 'bounty_owner_github_username', 'standard_bounties_id',
'application_scheme', 'work_scheme' ]:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E202 whitespace before ']'

@codecov
Copy link

codecov bot commented May 8, 2018

Codecov Report

Merging #1098 into master will decrease coverage by 0.36%.
The diff coverage is 16.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1098      +/-   ##
==========================================
- Coverage   30.64%   30.28%   -0.37%     
==========================================
  Files         124      125       +1     
  Lines        8790     8999     +209     
  Branches     1141     1155      +14     
==========================================
+ Hits         2694     2725      +31     
- Misses       5988     6166     +178     
  Partials      108      108
Impacted Files Coverage Δ
app/app/urls.py 94.28% <ø> (ø) ⬆️
app/dashboard/helpers.py 26.51% <ø> (ø) ⬆️
.../management/commands/pending_start_work_actions.py 0% <0%> (ø)
app/marketing/mails.py 12.08% <10%> (-0.31%) ⬇️
app/dashboard/signals.py 54.54% <100%> (-3.79%) ⬇️
app/dashboard/router.py 34.73% <100%> (ø) ⬆️
app/retail/emails.py 21.23% <15.38%> (-1.68%) ⬇️
app/dashboard/notifications.py 16.52% <3.03%> (-0.52%) ⬇️
app/dashboard/views.py 15.75% <5.17%> (-0.99%) ⬇️
app/dashboard/models.py 56.5% <57.14%> (+0.92%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5310da...8e1bf64. Read the comment docs.

@@ -514,7 +514,8 @@ var do_actions = function(result) {
pull_interest_list(result['pk'], function(is_interested) {

// which actions should we show?
var show_start_stop_work = is_still_on_happy_path;
var should_block_from_starting_work = !is_interested && result['work_scheme'] == 'traditional' && (result['status'] == 'started' || result['status'] == 'submitted')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing semicolon. (semi)

@PixelantDesign
Copy link
Contributor

Can we rename Application Scheme ---> Approval Process?

Do we want to encourage work contests?

@owocki
Copy link
Contributor Author

owocki commented May 8, 2018

Do we want to encourage work contests?

theres a whole long convo about this at gitcoinco/gitcoinco#9

@abitrolly
Copy link
Contributor

Enforcing limit that a person can not start ticket if the person is already doing other ticket.

},
'work_scheme': function(key, val, result) {
return [ 'work_scheme', result.work_scheme ];
return [ 'bounty_owner_name', result.bounty_owner_name ];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unreachable code. (no-unreachable)

schemes through start work flow
@@ -421,7 +429,8 @@ var show_interest_modal = function() {
var self = this;

setTimeout(function() {
$.get('/interest/modal?redirect=' + window.location.pathname, function(newHTML) {
var url = '/interest/modal?redirect=' + window.location.pathname + "&pk=" + document.result['pk']

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected blank line after variable declarations. (newline-after-var)
Strings must use singlequote. (quotes)
Missing semicolon. (semi)

@@ -392,21 +392,39 @@ def build_github_notification(bounty, event_name, profile_pairs=None):
"* Questions? Checkout <a href='https://gitcoin.co/help'>Gitcoin Help</a> or <a href='https://gitcoin.co/slack'>Gitcoin Slack</a>\n * " \
f"${amount_open_work} more funded OSS Work available on the [Gitcoin Issue Explorer](https://gitcoin.co/explorer)\n"
elif event_name == 'work_started':
interested = bounty.interested.all()
interestd_plural = "s" if interested.count() != 0 else ""

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

F841 local variable 'interestd_plural' is assigned to but never used

msg = f"{status_header}__Work has been started__.\n{profiles} has committed to working on this project to be " \
f"completed {from_now}.\n\n"
started_work = bounty.interested.filter(pending=False).all()
pending_approval = bounty.interested.filter(pending=True).all()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

F841 local variable 'pending_approval' is assigned to but never used


issue_message = interest.issue_message.strip()
if issue_message:
msg += f"{bounty_owner_clear} they have the following comments/questions for you:\n\n```{issue_message}```"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E501 line too long (123 > 120 characters)

@@ -190,14 +205,14 @@ def new_interest(request, bounty_id):

if profile.has_been_removed_by_staff():
return JsonResponse({
'error': _('Because a staff member has had to remove you from a bounty in the past, you are unable to start more work at this time. Please contact support.'),
'error': _('Because a staff member has had to remove you from a bounty in the past, you are unable to start more work at this time. Please leave a message on slack if you feel this message is in error.'),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E501 line too long (216 > 120 characters)

if is_funder or is_staff:
interests = bounty.interested.filter(pending=True, profile__handle=worker)
if not interests.exists():
messages.warning(request, _('This worker does not exist or is not in a pending state. Please check your link and try again.'))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E501 line too long (142 > 120 characters)

interest.pending = False
interest.save()

# TODO: send an email when

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W291 trailing whitespace

maybe_market_to_slack(bounty, 'worker_approved')
maybe_market_to_user_slack(bounty, 'worker_approved')
maybe_market_to_twitter(bounty, 'worker_approved')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W293 blank line contains whitespace

maybe_market_to_twitter(bounty, 'worker_approved')


else:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E303 too many blank lines (2)

@owocki
Copy link
Contributor Author

owocki commented May 20, 2018

mocked up github issue for starting work in the new work schemes paradigm http://bits.owocki.com/17161E2M1x18/Screen%20Shot%202018-05-20%20at%202.31.01%20PM.png

@owocki
Copy link
Contributor Author

owocki commented May 21, 2018

example gitcoinbot comment thread owocki/pytrader#96

@mbeacom
Copy link
Contributor

mbeacom commented May 21, 2018

screenshot 2018-05-21 11 25 06

Initial thoughts:

The copy: @owocki they have the following comments/questions for you: seems repetitive when applied to all users starting work.

Maybe we could restructure the comment to have less repetitive copy with something like:

Option 1

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

@owocki These users each claimed they can complete the work within 12 months from now. Please review their questions below:

  1. gitcointestuser has applied to start work (Funders only: approve worker | reject worker).

    • Q: foo the bar. lol 123
  2. gitcoinbot has been approved to start work.

    • Q: i am the walrus
  3. joesnuffy has been approved to start work.

    • Q: How do I get started?

Option 2

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

These users each claimed they can complete the work within 12 months from now. Please review their questions below:

Start Work

Awaiting Approval
  1. gitcointestuser (Funders only: approve worker | reject worker).
    • Q: foo the bar. lol 123
Approved
  1. gitcoinbot
    • Q: i am the walrus
  2. joesnuffy
    • Q: How do I get started?

Option 3

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

These users each claimed they can complete the work within 12 months from now:

  1. gitcointestuser has applied to start work (Funders only: approve worker | reject worker).
  2. gitcoinbot has been approved to start work.
  3. joesnuffy has been approved to start work.
Questions:

@owocki Please review the following questions:

  1. foo the bar. lol 123
  2. i am the walrus
  3. How do I get started?

@@ -414,7 +414,7 @@ def create_new_bounty(old_bounties, bounty_payload, bounty_details, bounty_id):
# Currently we are only considering the latest fulfillment. Std bounties supports multiple.
# If any of the fulfillments have been accepted, the bounty is now accepted and complete.
accepted = any([fulfillment.get('accepted') for fulfillment in fulfillments])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W293 blank line contains whitespace

@@ -178,6 +186,13 @@ def new_interest(request, bounty_id):
except Bounty.DoesNotExist:
raise Http404

is_too_many_already_started = bounty.work_scheme == 'traditional' and bounty.interested.filter(pending=False).count() > 0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E501 line too long (125 > 120 characters)

Copy link
Contributor

@mbeacom mbeacom May 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do:

is_too_many_already_started = bounty.work_scheme == 'traditional' and bounty.interested.filter(pending=False).exists()
to avoid the COUNT().

Also, can we add Bounty.SCHEME_TYPES and the individual scheme types, so: bounty.work_scheme == 'traditional' would become: bounty.work_scheme == Bounty.SCHEME_TRADITIONAL or something like that.

Bounty.work_scheme would be modified to include choices=SCHEME_TYPES

@owocki
Copy link
Contributor Author

owocki commented May 22, 2018

@mbeacom i like option number 1

@PixelantDesign
Copy link
Contributor

PixelantDesign commented May 23, 2018

Not to bloat up the work schemes, but I wonder if it might makes sense to allow for a progressive input under Work Scheme: Traditional so that the funder can tag a contributor that they're reserving the issue for.

Example: Reserved for @owocki

@owocki
Copy link
Contributor Author

owocki commented May 23, 2018

i like the idea.. can we v2 it?

@thelostone-mc
Copy link
Member

thelostone-mc commented May 23, 2018

@owocki @mbeacom could we consider option 2 over 1.
Sole reason being : It's neat segregated into two buckets and make things clear ! (No repetitive text )

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

@owocki These users each claimed they can complete the work within 12 months from now. Please review their questions below:

Awaiting Approval (Only funder can approve / reject worker)

  1. gitcointestuser (approve / reject) : foo the bar. lol 123

Approved

  1. gitcoinbot : i am the walrus
  2. joesnuffy : How do I get started?

thelostone-mc
thelostone-mc previously approved these changes Jun 4, 2018
Copy link
Member

@thelostone-mc thelostone-mc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -136,6 +138,15 @@ var callbacks = {
'bounty_owner_name': function(key, val, result) {
return [ 'bounty_owner_name', result.bounty_owner_name ];
},
'permission_type': function(key, val, result) {
if(val == 'approval'){

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected space(s) after "if". (keyword-spacing)
Missing space before opening brace. (space-before-blocks)

app/app/urls.py Outdated
@@ -238,6 +238,14 @@
url(r'^_administration/email/start_work_expired$', retail.emails.start_work_expired, name='start_work_expired'),
re_path(r'^_administration/email/gdpr_reconsent$', retail.emails.gdpr_reconsent, name='gdpr_reconsent'),
url(r'^_administration/email/new_tip/resend$', retail.emails.resend_new_tip, name='resend_new_tip'),
url(r'^_administration/process_accesscode_request/(.*)$', tdi.views.process_accesscode_request, name='process_accesscode_request'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make fix-yapf or make-fix

@@ -133,7 +138,10 @@ def gh_login(request):

def get_interest_modal(request):

bounty = Bounty.objects.get(pk=request.GET.get("pk"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we catch if request.GET.get('pk') is not present?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should always be present per the JS tho?


<h1>{% trans "24 Hrs to Approve" %}</h1>
<div style="text-align: left! important">
<p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we switch this to two-space indentation for all of the new html templates throughout?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me see how to auto lint this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

msg = f"{status_header}__Workers have applied to start work__.\n\n"

msg += f"\nThese users each claimed they can complete the work by {from_now}. " \
"Please review their questions below:\n\n"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E127 continuation line over-indented for visual indent

@@ -934,12 +983,21 @@ class Interest(models.Model):
"""Define relationship for profiles expressing interest on a bounty."""

profile = models.ForeignKey('dashboard.Profile', related_name='interested', on_delete=models.CASCADE)
created = models.DateTimeField(auto_now_add=True, blank=True, null=True)
created = models.DateTimeField(auto_now_add=True, blank=True, null=True)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W291 trailing whitespace

$('.js-select2').each(function() {
$(this).select2();
});
//removes tooltip

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected space or tab after '//' in comment. (spaced-comment)

$('.js-select2').each(function() {
$(this).select2();
});
//removes tooltip
$('select').on('change', function (evt) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected space before function parentheses. (space-before-function-paren)

$('.js-select2').each(function() {
$(this).select2();
});
//removes tooltip
$('select').on('change', function (evt) {
$('.select2-selection__rendered').removeAttr('title');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected indentation of 4 spaces but found 6. (indent)

$('select').on('change', function (evt) {
$('.select2-selection__rendered').removeAttr('title');
});
//removes search field in all but the 'denomination' dropdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected space or tab after '//' in comment. (spaced-comment)

$('.select2-selection__rendered').removeAttr('title');
});
//removes search field in all but the 'denomination' dropdown
$('.select2-container').click(function(){

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space before opening brace. (space-before-blocks)

});
//removes search field in all but the 'denomination' dropdown
$('.select2-container').click(function(){
$('.select2-container .select2-search__field').remove();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected indentation of 4 spaces but found 6. (indent)

$('.select2-container').click(function(){
$('.select2-container .select2-search__field').remove();
});
//denomination field

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected space or tab after '//' in comment. (spaced-comment)

@owocki owocki merged commit 8e1bf64 into master Jun 4, 2018
@ghost ghost removed the in progress label Jun 4, 2018
@mbeacom mbeacom deleted the kevin/application_schemes branch June 4, 2018 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants